Add owner-authorized manual workflow triggers - #6964

Review verdict: request changes

Reviewed exact head 143744f8d8768b7209f65a9486580d3d89b228ec against base 92f15c042a3e314483bcb5478f9525b81681f2c2.

1. Major — Desktop retry does not preserve the operation identity after ambiguous delivery

desktop/src-tauri/src/commands/workflows.rs:321-341 rebuilds a trigger for each command invocation. The builder generates a fresh random request ID (crates/buzz-sdk/src/builders.rs:1643-1678), while submit_event keeps the signed event only for the current call and drops it on transport/status/parse failure (desktop/src-tauri/src/relay/submit.rs:16-50,58-78). Relay duplicate recovery is keyed to the exact event ID (crates/buzz-relay/src/handlers/command_executor.rs:992-1005).

If the relay commits a trigger/run but its response is lost, Desktop reports failure; the user's retry builds a different event and can execute a second run, duplicating messages, webhooks, or other external effects. This defeats the retry/idempotency contract added by the PR.

Author action: retain and resubmit the exact signed trigger across outcome-ambiguous failures. If restart recovery is promised, journal it durably and scope it to relay/community/identity. Add a production-seam regression that commits the first request, loses its response, retries through Desktop, and asserts one event ID, one run ID, and one side effect; mutation-prove that rebuilding on retry fails the test.

Verification owner: author for implementation/regression; reviewer for exact-head renderer → Tauri → relay confirmation.

2. Moderate — Primary Desktop trigger paths silently fail and animate before settlement

The main card/editor paths invoke the mutation without error handling (desktop/src/features/workflows/ui/WorkflowsView.tsx:155-162,208-212; desktop/src/app/AppWorkflowEditorOverlayProvider.tsx:98-105,118-122), and the card starts its action animation before the request settles (desktop/src/features/workflows/ui/WorkflowCard.tsx:285-288). The explicit alert in WorkflowDetailPanel.tsx:144-154 does not cover these primary entry points. Current E2E counts the mocked command invocation, bypassing revision fetch, signing, and relay rejection/acceptance (desktop/tests/e2e/workflows.spec.ts:1042-1063; desktop/src/testing/e2eBridge.ts:4032-4043).

A stale revision, disabled workflow, revoked authority, or network failure therefore produces no actionable user-visible result while motion implies the workflow fired, encouraging retries and amplifying finding 1.

Author action: provide accessible pending/error/retry state on every trigger entry point and animate success only after confirmed run creation. Add rejection/retry coverage beyond the mock invocation counter, including at least one revision GET → signing → relay acceptance/rejection integration path.

Verification owner: author for behavior/tests; reviewer for product and integration verification.

Validation and residual risk

Two independent review lanes traced the production seam and confirmed both findings. On the same clean head, package runs passed for buzz-sdk, buzz-cli, and buzz-workflow (workflow: 169 passed, 2 ignored); git diff --check passed. GitHub currently reports 31 successful and 6 skipped checks, including PostgreSQL, relay/backend E2E, Desktop core/integration, macOS, and Windows. Green CI does not exercise the committed-response-loss retry boundary above.

Native GUI/VoiceOver observation was not run on the shared machine; this is a reviewer confidence gap, not separate author rework. A possible crash window after run commit and before spawned execution remains unproven and is not an additional blocker in this review.

Review verdict: request changes

Reviewed base 92f15c042a3e314483bcb5478f9525b81681f2c2 through exact live head 143744f8d8768b7209f65a9486580d3d89b228ec, integrating both assigned lanes. Both independently confirmed two author-actionable defects.

1. Desktop retries can execute a workflow twice after an ambiguous committed delivery

Desktop builds a trigger with a fresh randomized request-id and immediately performs one-shot submission (desktop/src-tauri/src/commands/workflows.rs:321-341; crates/buzz-sdk/src/builders.rs:1643-1678). The signed event exists only in that call stack and is discarded on transport/status/parse error (desktop/src-tauri/src/relay/submit.rs:16-50,58-78). A user retry rebuilds a different event. Relay duplicate recovery, however, is keyed to the exact original event ID (crates/buzz-relay/src/handlers/command_executor.rs:992-1005).

If the relay commits the first run but its response is lost, Desktop reports failure; retry creates a second run and can duplicate message/webhook/external effects. The server idempotency mechanism is sound, but the shipped caller cannot use it at the outcome-ambiguous boundary.

Author action: retain and resubmit the exact signed trigger across ambiguous outcomes, durably if restart recovery is promised. Add a production-seam committed-response-loss regression asserting one event ID, one run ID, and one side effect; mutation-prove rebuilding on retry fails.

2. Primary Desktop trigger paths silently fail and animate before settlement

The card/editor entry points call mutate without error handling (desktop/src/features/workflows/ui/WorkflowsView.tsx:155-162,208-212; desktop/src/app/AppWorkflowEditorOverlayProvider.tsx:98-105,118-122), while WorkflowCard starts its action animation before the request settles (WorkflowCard.tsx:285-288). Stale revision, disabled/revoked authority, preflight, and network rejection therefore produce no actionable status while the UI implies firing—encouraging the dangerous retry above.

Author action: expose accessible pending/error/retry state on every trigger entry point, animate success only after confirmed run creation, and add rejection/retry coverage that binds revision fetch → Tauri signing → relay response rather than only a mock invocation counter.

Verification

Exact-head SDK/CLI/workflow package suites passed (workflow 169 passed, 2 ignored; SDK 304 passed). Relevant exact-head CI is green. Native GUI/VoiceOver and locally ignored PostgreSQL cases remain reviewer/CI confidence gaps, not additional author work.

— :bot: Jude’s code review agent

Request changes

I found two remaining recovery-contract defects at exact head 547a6ef1dcf598cb3cb264e84fe27c90f109363d.

1. Exact-event recovery expires before the Desktop retry contract does

The Desktop retains the signed event for session-lifetime retry (desktop/src/features/workflows/triggerOperations.ts:21-23,87-95), but production ingest applies the ±15-minute freshness rejection first (crates/buzz-relay/src/handlers/ingest.rs:2234-2240,2286-2289). Duplicate recovery for an already committed trigger is reached only afterward (crates/buzz-relay/src/handlers/command_executor.rs:992-1004).

Consequently, if the relay commits a trigger but its response is lost, retrying the retained exact event after 15 minutes cannot recover the original acknowledgment. Starting a distinct run can then duplicate already committed effects. The current native response-loss test submits against a mock HTTP ledger, while relay replay tests call the command handler directly; neither crosses the production ingest freshness gate.

Required: allow an exact replay of an already committed manual-trigger event to recover its original run after the normal freshness window while stale unseen events remain rejected, or replace/bound the UI promise with a safe reconciliation contract. Add an ingest-level regression covering commit → lost response → >15 minutes → exact-byte replay, asserting the same event/run and one effect, plus a stale-unseen negative control.

2. Pre-submit failures are presented as ambiguous committed submissions

WorkflowTriggerOperations.run() assigns the same error state when preparation fails before an event exists and when submission fails after publication may have committed (desktop/src/features/workflows/triggerOperations.ts:82-103). The shared feedback then says the run may already exist, claims Retry reuses the same signed request, and offers “Start a distinct run…” (desktop/src/features/workflows/ui/WorkflowTriggerFeedback.tsx:37-72). For revision lookup, stale-revision preparation, or signing failure, those statements are false: there is no retained or submitted event, and the existing operation test confirms preparation runs again.

Required: distinguish pre-submit failure from ambiguous post-submit failure (for example with an event-retained/submission-attempted discriminator). Pre-submit failures should say the run was not started and offer an ordinary retry; reserve duplicate-run warnings and abandonment for attempted submissions. Add shared production-seam UI coverage for both phases, including evidence that the editor entry point consumes the behavior.

Validation and residual risk

• Live PR head rechecked as 547a6ef1dcf598cb3cb264e84fe27c90f109363d; base d2a80d819979efbf4b02e8b322a425c91c18e046.
• Clean detached review worktrees; git diff --check d2a80d8...547a6ef passed.
• Full Desktop test suite passed: 6,240/6,240. Desktop TypeScript and pnpm check also passed (pre-existing warnings outside changed files).
• The prior blockers did materially improve: exact signed bytes are retained/coalesced, operation scope includes community/signer/workflow, all three UI entry points share pending/error/success state, and card animation now follows settlement.
• Native GUI/VoiceOver and a unified renderer → native → live relay journey were not observed. Native Tauri tests were blocked locally by the absent required sidecar. Several exact-head CI jobs were still running when this review was submitted. These are confidence gaps, not the basis for this request-changes verdict.

Author action: resolve both defects and add the regressions above. Verification owner: author for implementation/tests; reviewer for exact-head ingest replay, causal mutation, and UI-state re-review.

Verdict: REQUEST CHANGES

Reviewed: d2a80d819979efbf4b02e8b322a425c91c18e046..547a6ef1dcf598cb3cb264e84fe27c90f109363d (exact head 547a6ef1dcf598cb3cb264e84fe27c90f109363d)

Risk: high — manual workflow execution crosses renderer, Tauri signing/submission, relay ingest/idempotency, authorization, and externally visible side effects.

Behavior/contracts traced: card/detail/editor trigger state; shared-operation scoping and coalescing; preparation versus submission; exact signed-event retention; response-loss retry; community/account switching; relay freshness admission and duplicate recovery; atomic trigger/run persistence; stale revision and authority rejection; success animation and accessible recovery UI.

Blocking findings

1. [P1] Permit safe exact replay recovery after the normal freshness window. Desktop intentionally retains the exact signed trigger for session-lifetime recovery (desktop/src/features/workflows/triggerOperations.ts:21-23,87-95), but production ingest rejects events outside ±15 minutes before command duplicate recovery runs (crates/buzz-relay/src/handlers/ingest.rs:2234-2240,2286-2289; duplicate recovery is later at crates/buzz-relay/src/handlers/command_executor.rs:992-1004). A trigger can commit, lose its response, then become unrecoverable when Retry is used after 15 minutes; abandoning and starting a distinct run can duplicate already-committed effects. Current native response-loss and relay replay tests bypass production ingest, so they cannot detect this ordering defect.

   Author action: allow an exact replay of an already-committed manual-trigger event to recover its original acknowledgment after the ordinary freshness window while stale unseen events remain rejected, or replace the UI contract with an equally safe reconciliation path. Add an ingest-level commit → lost response → >15-minute exact replay regression asserting the same event/run and one effect, plus a stale-unseen negative control; mutation-prove the admission ordering.

   Verification owner: author for implementation/regression; reviewer for exact-head ingest-level causal verification.

2. [P2] Distinguish pre-submit failure from ambiguous post-submit failure. WorkflowTriggerOperations.run() assigns one error state to both prepareWorkflowTrigger failures—before any signed event exists—and triggerWorkflow failures after submission may have committed (desktop/src/features/workflows/triggerOperations.ts:82-103). Every shared surface consequently says the run may already exist, claims Retry reuses the same signed request, and offers a distinct-run abandonment choice (desktop/src/features/workflows/WorkflowTriggerFeedback.tsx:37-72). For stale-revision lookup, signing, or other preparation failure, those claims are false; the existing test confirms preparation is attempted again (triggerOperations.test.mjs:111-124).

   Author action: represent pre-submit and ambiguous-submit failures separately (or carry an equivalent retained-event/submission-attempt discriminator). Pre-submit failures must state that execution did not start and offer ordinary retry without duplicate-run warnings; preserve exact-event retry and abandonment warnings only after submission was attempted. Cover both phases through the shared production UI seam, including editor applicability.

   Verification owner: author for implementation/regression; reviewer for changed-head product verification.

The prior blockers are otherwise materially repaired: renderer operations retain exact signed bytes and coalesce by relay/signer/workflow scope; native code captures relay/signer snapshots; card/detail/editor share pending/error/success state; card animation follows successful settlement; trigger event and run persistence remain atomic and tenant/workflow scoped. Restart durability is explicitly out of scope rather than falsely promised.

Validation at matching exact head: both independent lanes used clean detached 547a6ef1dcf598cb3cb264e84fe27c90f109363d trees with merge-base d2a80d819979efbf4b02e8b322a425c91c18e046; git diff --check passed; full Desktop JS package passed 6,240/6,240; Desktop TypeScript passed; affected static checks passed with only unrelated existing warnings.

Manual/native evidence: no native GUI/VoiceOver journey. Local Tauri package execution was blocked before tests by the absent required sidecar. These are confidence gaps, not the basis for REQUEST CHANGES.

Residual risk: final-head CI was still completing at lane close; exact native renderer → Tauri → live-relay behavior and restart boundaries remain unobserved. CI/reviewer tooling owns those gaps unless a PR-caused failure emerges.

— :bot: Jude’s code review agent

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reconsidered: exact unchanged head 547a6ef1dcf598cb3cb264e84fe27c90f109363d after the author’s substantive response #6964 (comment) (5540176733, 2026-09-04 12:05:27 UTC).

Both assigned lanes independently rechecked the published object. The response confirms both blockers and describes only an unpublished partial UI repair; it does not clear either author action.

1. Expired committed-event recovery remains unreachable. Production ingest rejects timestamp drift over 900 seconds before command dispatch (crates/buzz-relay/src/handlers/ingest.rs:2234-2240,2286-2289), while exact committed-trigger recovery occurs later (crates/buzz-relay/src/handlers/command_executor.rs:992-1004). A session-retained exact replay after 15 minutes therefore cannot recover its original run, and starting a distinct run can duplicate effects.

   Author action: publish a read-only exact committed-event reconciliation path that preserves signature, identity, event shape, community/workflow/event scope, and current authority checks without admitting unseen stale work. Add production-ingest tests proving expired exact replay returns the original event/run with one effect and expired unseen input creates nothing; mutation-prove the ordering.

2. Published UI still conflates pre-submit and ambiguous post-submit failure. desktop/src/features/workflows/triggerOperations.ts:82-103 emits one undifferentiated error state; WorkflowTriggerFeedback.tsx:37-72 consequently tells preparation/signing failures that a run may exist, claims retry reuses a retained signed request, and offers distinct-run abandonment even when nothing was submitted.

   Author action: publish the phase-aware repair, reserve ambiguous-run/exact-event/abandonment language for attempted submission, and add causal shared production-seam coverage for preparation rejection versus committed response loss, demonstrably binding all entry points.

Verification owner: author for published fixes and regressions; reviewer/A-Team for the next exact-head ordering, stale-unseen fencing, UI phase behavior, and causal mutation review.

Validation / confidence: live head/base remained 547a6ef1… / d2a80d8…; exact-head CI is terminal with 50 successful and 23 skipped checks, none pending or failed. Green CI closes the prior gate-confidence gap but does not alter these source-confirmed defects. The unpublished 07e4fd21… was not treated as review evidence.

Request changes

Reviewed exact head 4ab34922d86bc8fb11b2cab2b7447a80f5a85d30 against base d2a80d819979efbf4b02e8b322a425c91c18e046. The previous recovery and phase-accuracy implementations are substantially repaired, but two author-actionable gaps remain.

1. Expired recovery bypasses NIP-42 channel-token authorization

ingest_event_inner routes expired kind-46020 events to recover_workflow_trigger after signature, identity, and coarse MessagesWrite checks (crates/buzz-relay/src/handlers/ingest.rs:2275-2306). This early return occurs before normal channel derivation and check_token_channel_access (ingest.rs:2424-2508). The recovery handler resolves the workflow/channel and validates ownership/current authority, but does not enforce auth.channel_ids() (crates/buzz-relay/src/handlers/command_executor.rs:947-1039).

A NIP-42 token restricted to channel A can therefore replay and recover the run ID for its controlled workflow in channel B, although the ordinary ingest path rejects that token for channel B. Existing trigger-ingest tests use IngestAuth::Http, which cannot carry channel restrictions, so they do not exercise this skipped fence.

Required: enforce the recovered workflow channel using the same token-channel access contract before committed lookup, or route recovery through an equivalent shared fence. Add production-ingest coverage using IngestAuth::Nip42 { channel_ids: Some(...) }: a wrong-channel token must reject with no mutation/effect; a matching-channel token must recover the original event/run. Removing the check must fail the test.

2. The editor's phase-aware feedback binding is not causally tested

The UI implementation now correctly distinguishes preparation failure from ambiguous submission failure, and card/detail/editor source all bind the shared state. However, desktop/src/features/workflows/ui/workflowTriggerFeedback.test.mjs:45-259 renders and asserts only WorkflowCard and WorkflowDetailPanel, not WorkflowDialog. Replacing the editor's state={trigger} binding at WorkflowDialog.tsx:678-682 with idle state still leaves the full Desktop suite green (6,240/6,240). Thus the editor can silently lose the required phase-accurate retry/abandonment behavior without any regression failing.

Required: render/open the production editor host (or an equally direct production seam), drive preparation rejection and ambiguous committed-response loss, and assert mutually exclusive copy/actions plus exact-event retry. Removing the editor binding must fail behaviorally.

What cleared

• Expired trigger recovery is now a committed-only, read-only branch after signature/identity/coarse-scope checks; stale unseen events cannot insert or spawn through that branch.
• The recovery lookup validates event shape, tenant/workflow binding, controlling identity, revision, lifecycle/current authority, retained event binding, and scoped event/run identity.
• Preparation failures now say no run started and omit abandonment; ambiguous submission failures retain exact-event retry and duplicate-side-effect warnings. Card, detail, and editor source use the shared feedback.

Validation and residual risk

• Live head/base rechecked immediately before review: 4ab34922… / d2a80d8…; GitHub identity jedwards27 differs from author loganj.
• CI is terminal: 50 successful, 23 skipped, no pending or failed checks.
• Clean detached review worktrees; git diff --check passed.
• Desktop: just desktop-test 6,240/6,240; typecheck, build, file-size, and check passed (only pre-existing warnings outside changed files).
• Relay: 1,034 passed / 104 ignored / 1 failed locally; the isolated mesh-demo echo test reproduced a 504 and is outside this delta while exact-head CI is green, so it is a confidence gap rather than author rework.
• PostgreSQL mutation execution and native GUI/VoiceOver observation were unavailable locally. These are residual verification gaps, not the basis for this verdict.

Author action: resolve both gaps and add the causal regressions above. Verification owner: author for implementation/tests; reviewer for exact-head authorization ordering, mutation proof, and editor production-seam re-review.